WIP: os/posix: port of the posix implementation to macOS #352

stanislaw · 2020-01-20T18:00:41Z

Describe the contribution

This is an ongoing attempt to make the CFS / OSAL work on macOS. I am opening this is as a Work-in-Progress Pull Request because there are many things that have to be addressed.

Rough plan of the work (major points):

Some important comments:

It is not the first time that I am trying to make something work on macOS so I had some code in my pockets already. I have collected the POSIX functions which were missing on macOS to a separate project: posix-mac-addons. For now, I am simply copying the src contents of that project to the root of the osal repository. Please see the comments on the implemented functions on that project's README page as well.
I have built this branch from 2 different macOS machines and I can confirm that everything seems to work on my end: building them from a clean tree and running the tests. Both are Mojave though, so I cannot promise that everything will work on Catalina (many things are breaking for those who have upgraded so upgrading this changeset to Catalina could be a separate action).
All of the tests as found in the changeset's Makefile are passing. One could use this Makefile as a starting point for working with this changeset.
I was not sure whether I should have created the posix-mac port or simply hack on top of posix. I decided to go for latter because it is now easier to see on the diff what is different. However, if there is an interest in getting this merged, it is not clear to me what would be the right solution: keep posix with lots of ifs or create a dedicated mac-posix one.
Both implementations of the semaphores are passing the tests. This changeset is using the one which is mac_sem2_* based on the GCD semaphores. The following important detail explains why the implementation is a bit more sophisticated. Without this detail some of the tests are crashing. I guess, this could be fixed by simply ensuring that the semaphores in the tests are used in a balanced way.

GCD semaphores crash with `BUG IN CLIENT OF LIBDISPATCH` when you destroy a
semaphore which is being waited on this is why in the `mac_sem2_*`
implementation there is a layer that tracks how many times a certain semaphore
was waited/posted to release the semaphore manually when it gets destroyed. This
behavior should be disabled in the future and was only needed to make one
important project's tests to pass without crashing.
https://github.com/stanislaw/posix-mac-addons/tree/a8c406c65cf82007ab1e7cc126cc3bf142facf0a#known-issue

Testing performed

The testing has been performed on macOS Mojave 10.14.6

Expected behavior changes

System(s) tested on

Hardware: MacBook Pro (Retina, 15', Mitte 2015)
OS: macOS Mojave 10.14.6
Versions: This branch only

Additional context

Everything is in the description.

Third party code

This is to be defined very soon. 2/3 of the code in posix-mac-addons is created because of my work on this port but I would really like to have posix-mac-addons available as a standalone repository that will have a MIT license. I would like to know your thoughts on if / how this third-party code could be integrated then.

Contributor Info - All information REQUIRED for consideration of pull request

Stanislav Pankevich, personal

The signed individual CLA has been sent to the email specified in the CLA document.

jphickey · 2020-01-20T19:48:56Z

Wow... lots to digest in here. I took a quick skim through and it seems there are quite a few different things that this addressing. It would be helpful if this could somehow be broken into smaller, more easily digestible change sets for review.

We will have to have some discussions on with the broader user community on the right way to proceed here. I was hoping that most of the changes could be done in such a way where it would be limited to some small variation on the POSIX implementation.

stanislaw · 2020-01-22T17:33:29Z

Wow... lots to digest in here. I took a quick skim through and it seems there are quite a few different things that this addressing. It would be helpful if this could somehow be broken into smaller, more easily digestible change sets for review.

Yes, it is a lot. And this is why I have annotated every change with a comment which explains why I have to make it. It would be possible to reduce the size of the diff to certain extent but I am afraid that most of it is taken by the posix-mac-addons and without those files the code simply does not compile on macOS.

I can comment on the changeset on everything where I think the parts could be considered standalone and if these parts are confirmed I can create corresponding separate Pull Requests for them. I will start doing comments now.

We will have to have some discussions on with the broader user community on the right way to proceed here. I was hoping that most of the changes could be done in such a way where it would be limited to some small variation on the POSIX implementation.

I would like to get at least one confirmation from a macOS user that this branch works fine for them. From my side, I am fully committed to making this work ready to be merged if there is a general interest in having a macOS port in the tree.

stanislaw · 2020-01-22T17:36:42Z

Makefile

@@ -0,0 +1,34 @@
+
+
+test: test.unit test.integration


This is the first obvious candidate for removal or extraction. I have not found a single place in the osal from where I can run ALL unit and integration tests. I suspect that there is some private branch of nasa/osal where such script exists.

Having such a "one-click" solution for running all of the tests is essential for making sure that the osal is not broken when making changes and testing them on both Linux and macOS.

Please advise if I should remove this custom Makefile and rather use some existing test scripts.

Looks like this will satisfy it: #403.

stanislaw · 2020-01-22T17:39:15Z

posix-mac-addons/CMakeLists.txt

@@ -0,0 +1,22 @@
+cmake_minimum_required(VERSION 3.13)


I will not comment on any of the posix-mac-addons now. The cleanups and further improvements are tracked here: https://github.com/stanislaw/posix-mac-addons/issues. For the time being, I will be force-pushing or adding some cleanups to this branch while also re-running the tests with the test Makefile from this changeset.

src/bsp/pc-linux/src/bsp_start.c

src/os/shared/osapi-module.c

stanislaw · 2020-01-22T17:44:25Z

src/unit-tests/osloader-test/CMakeLists.txt

+    # TODO-MAC:
+    # Forcing eeprom1 to be in CMAKE_BINARY_DIR.
+    LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/eeprom1)
+  target_link_libraries(osal_loader_UT MODULE${MOD})


This specific line target_link_libraries... can be extracted to a separate PR also with the add_osal_ut_exe line moved above like on the diff.

The LIBRARY_OUTPUT_DIRECTORY will be a separate discussion.

src/os/posix/osapi.c

stanislaw · 2020-01-23T21:54:32Z

I have updated this PR's description as well. It is also my individual contribution.

stanislaw · 2020-04-21T14:25:09Z

src/bsp/pc-linux/build_options.cmake

@@ -18,8 +18,8 @@ target_link_libraries(osal_bsp
 # Note - although GCC understands the same flags for compile and link here, this may
 # not be true on all platforms so the compile and link flags are specified separately.
 if (NOT CMAKE_CROSSCOMPILING)
-  set(UT_COVERAGE_COMPILE_FLAGS -pg --coverage)


Opened here: #420.

stanislaw · 2020-04-21T14:26:49Z

src/os/portable/os-impl-posix-dl.c

+    /// Current working dir: /sandbox/cFS/osal/build.commandline.dir
+    /// [ FAIL] 04.004 ut_osloader_symtable_test.c:186 - #4 Nominal
+    /// [  END] 04 OS_SymbolLookup      TOTAL::4     PASS::3     FAIL::1      MIR::0      TSF::0      N/A::0
+    Function = dlsym((void *)OSAL_DLSYM_DEFAULT_HANDLE, SymbolName);


stanislaw · 2020-04-22T13:06:39Z

src/unit-tests/osloader-test/CMakeLists.txt

@@ -4,7 +4,9 @@ set(TEST_MODULE_FILES
  ut_osloader_module_test.c
  ut_osloader_symtable_test.c    
  ut_osloader_test.c)
-
+
+add_osal_ut_exe(osal_loader_UT ${TEST_MODULE_FILES})


Opened as a separate one here: #431.

stanislaw · 2020-04-24T14:36:02Z

@jphickey @skliper I would like to hear your thoughts on the following topic:

I know that you are hesitant to consider merging this as a separate posix-mac port because of the longer-term plan of making a single portable POSIX implementation which would work on Linux/macOS/BSD.

However I have to work with cFS in a real development rather soon (in one or two months) and I will have to maintain both Linux and macOS support for our cFS-based software and so far I could not find an easy way of keeping both Linux and macOS variants of src/os/posix in a convenient way so that switching between them would be easy.

I have removed the posix-macos-addons from the current diff (now it is a gitignored folder, could be a submodule) so that you can see that the actual diff is very small. I am wondering if you could advise me on what would be the easiest way to integrate this diff as a separate port: posix-mac that would reuse most of the existing posix stuff but also have these custom couple of files macOS-specific.

Any advice on how I could integrate it in my downstream private fork would be highly appreciated.

Thank you.

jphickey · 2020-04-24T14:49:06Z

@stanislaw you'll want to check out the merges that are going into PR #427 first. This breaks up all the impl subsystems (queues, timers, mutexes, etc) into separate subsystems in separate source files. That means rather than maintaining patches to files, you should be able to just maintain entirely separate files (i.e. a separate mac-queue or mac-timer source file perhaps) and select which one you build based on CMAKE variables. I think this will be easier for you.

As far as a longer term solution goes, I think there is probably community interest in getting OSX support "out of the box" in the framework, but that's at least a few months out as NASA has other near-term objectives to meet right now.

But I do think making these into separate subsystem-level source files will help a lot in the near term, not only for your needs but also if/when we do merge this sometime in the future we can review each subsystem independently.

skliper · 2020-10-19T18:52:36Z

Heads up - format cleanup is likely coming soon which will create many merge conflicts for any pending PRs. This one looks like a significant number of merge conflicts have built up already...

stanislaw · 2020-10-19T19:05:57Z

@skliper thanks for the heads up. I could try rebasing the whole PR on top of the latest master but let me know if it is worth doing this.

I saw some hesitation by @jphickey about integrating this PR as is (this comment: #352 (comment)). My understanding is that even if I polished this PR to green it would still not be considered mergeable because the POSIX addons for macOS are something you maintainers would prefer to not integrate as a macOS-specific and therefore too custom OSAL and instead you want to spend an indefinite amount time on making the POSIX OSAL portable enough to accomodate for Linux AND BSD* including macOS.

On my side, I am happy to volunteer here and do whatever it takes to integrate these changes into the cFS tree. Just let me know if there is a way of doing this using this changeset.

jphickey · 2020-10-20T13:40:50Z

I think there are users out there that would definitely appreciate having some OSX support.... but the NASA projects that are currently pushing CFE/OSAL development don't require it, so it will likely have to be a volunteer effort.

The main issues to solve here are integration and some form of a test/CI plan. Integration is a big one - we can't break POSIX (or any other platform). And if we are to support this platform we need a plan to validate it - we already have platforms which are not tested on a regular basis, and as a result we get breakage on those platforms.

Doing a "clone and own" approach is fast and easy - but yields a result that is hard to maintain because its constantly falling behind its counterpart - especially if there isn't a CI plan in place. That's the reason I push for minimal duplication - reuse all code where possible, clone/modify only the specific subsections you really need to change. The specifics of how the code is organized (i.e. whether part of POSIX or a separate OSX layer) isn't as important - we can go either way - as long as the result is well integrated and maintainable/testable.

stanislaw · 2020-10-24T07:00:03Z

@jphickey

Thank you for your answer. I totally agree with everything you have said.

I could do the following exercise when I find a good time slot for it in the near future (1-2 weeks):

Enable CI for this macOS related osal using GitHub Actions or Travis CI. I would iterate on this branch in my own fork of osal to get through the setup steps and failing builds.
Create separate posix-macos layer for now with a customized CMake that will mostly point to the normal POSIX files. This should make the changeset rather small.
Connect posix-macos-addons repo as a submodule, for now, to reduce the size of the diff so that it is hopefully easier to review.
Show you the resulting branch and from there we will see if this work has a chance to be integrated into the main osal or it should rather be maintained as a downstream third-party repository.
In the case of a third-party package variant, I could see myself maintaining it for a while so that could be a working option.

Let me know if this sounds right to you.

thesamprice · 2021-08-25T18:47:40Z

I looked into a mac port a long time ago.
https://github.com/thesamprice/osal/tree/apple/src/os/posix
I implemented the queue system using memory buffers, and mutexes.
Very much a wip.

I then started looking at using qt to abstract the system calls.
https://github.com/thesamprice/osal/tree/apple/src/os/qt
I didnt get very far. Unsure if this is useful.

stanislaw · 2021-09-22T12:26:37Z

Closing this in favour of #1161.

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch 3 times, most recently from 56fd998 to 9e52bca Compare January 20, 2020 19:18

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch from 9e52bca to 0f0857d Compare January 21, 2020 22:46

stanislaw commented Jan 22, 2020

View reviewed changes

src/bsp/pc-linux/src/bsp_start.c Outdated Show resolved Hide resolved

stanislaw commented Jan 22, 2020

View reviewed changes

src/os/shared/osapi-module.c Outdated Show resolved Hide resolved

stanislaw commented Jan 22, 2020

View reviewed changes

src/os/posix/osapi.c Outdated Show resolved Hide resolved

skliper added the CCB:PendingCLA External contribution pending CLA confirmation label Jan 22, 2020

stanislaw mentioned this pull request Jan 22, 2020

os/shared/osapi-module.c: the return value of the call to OS_ModuleLoad_Impl should be checked #359

Closed

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch 2 times, most recently from 254bd63 to a5d2ffd Compare January 23, 2020 16:39

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch 3 times, most recently from 4e0b254 to d60891f Compare January 24, 2020 20:57

astrogeco added the CCB:Ignore Incomplete Pull Request with open actions. label Feb 11, 2020

astrogeco removed the CCB:Ignore Incomplete Pull Request with open actions. label Feb 19, 2020

skliper removed the CCB:PendingCLA External contribution pending CLA confirmation label Feb 21, 2020

stanislaw mentioned this pull request Feb 21, 2020

RFC: bin2cfetbl.py tool (ELF and Mach-O support) nasa/elf2cfetbl#34

Closed

9 tasks

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch from d60891f to fe22563 Compare February 27, 2020 21:09

stanislaw mentioned this pull request Feb 27, 2020

"duplicate symbol '_main'" on macOS when building tests #363

Closed

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch 4 times, most recently from 69f92d8 to 6011517 Compare April 21, 2020 10:45

stanislaw commented Apr 21, 2020

View reviewed changes

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch from 56eeb61 to f235a8a Compare April 21, 2020 14:41

stanislaw mentioned this pull request Apr 21, 2020

When running standalone, osal_loader_UT test binary file is missing test module files #430

Closed

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch 2 times, most recently from 698a2b3 to 0f0969e Compare April 22, 2020 13:03

stanislaw commented Apr 22, 2020

View reviewed changes

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch 3 times, most recently from a2e317a to 8c1b8ff Compare April 22, 2020 16:13

os/posix: port of the posix implementation to macOS

329ea9c

stanislaw force-pushed the posix-mac-20200120-after-rebase-with-master branch from 8c1b8ff to 329ea9c Compare April 22, 2020 16:15

skliper added the CCB:Ignore Incomplete Pull Request with open actions. label Apr 28, 2020

skliper added the enhancement label May 16, 2020

astrogeco marked this pull request as draft June 24, 2020 14:44

astrogeco closed this Oct 13, 2020

astrogeco reopened this Oct 13, 2020

astrogeco changed the base branch from master to main October 13, 2020 21:32

stanislaw mentioned this pull request Sep 22, 2021

os/posix: port of the posix implementation to macOS (take 2) #1161

Closed

5 tasks

stanislaw closed this Sep 22, 2021

skliper added duplicate and removed CCB:Ignore Incomplete Pull Request with open actions. labels Sep 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: os/posix: port of the posix implementation to macOS #352

WIP: os/posix: port of the posix implementation to macOS #352

stanislaw commented Jan 20, 2020 •

edited

Loading

jphickey commented Jan 20, 2020

stanislaw commented Jan 22, 2020

stanislaw Jan 22, 2020 •

edited

Loading

stanislaw Apr 22, 2020

stanislaw Jan 22, 2020

stanislaw Jan 22, 2020 •

edited

Loading

stanislaw commented Jan 23, 2020

stanislaw Apr 21, 2020

stanislaw Apr 21, 2020

stanislaw Apr 22, 2020

stanislaw commented Apr 24, 2020 •

edited

Loading

jphickey commented Apr 24, 2020

skliper commented Oct 19, 2020

stanislaw commented Oct 19, 2020

jphickey commented Oct 20, 2020

stanislaw commented Oct 24, 2020

thesamprice commented Aug 25, 2021

stanislaw commented Sep 22, 2021

WIP: os/posix: port of the posix implementation to macOS #352

WIP: os/posix: port of the posix implementation to macOS #352

Conversation

stanislaw commented Jan 20, 2020 • edited Loading

jphickey commented Jan 20, 2020

stanislaw commented Jan 22, 2020

stanislaw Jan 22, 2020 • edited Loading

Choose a reason for hiding this comment

stanislaw Apr 22, 2020

Choose a reason for hiding this comment

stanislaw Jan 22, 2020

Choose a reason for hiding this comment

stanislaw Jan 22, 2020 • edited Loading

Choose a reason for hiding this comment

stanislaw commented Jan 23, 2020

stanislaw Apr 21, 2020

Choose a reason for hiding this comment

stanislaw Apr 21, 2020

Choose a reason for hiding this comment

stanislaw Apr 22, 2020

Choose a reason for hiding this comment

stanislaw commented Apr 24, 2020 • edited Loading

jphickey commented Apr 24, 2020

skliper commented Oct 19, 2020

stanislaw commented Oct 19, 2020

jphickey commented Oct 20, 2020

stanislaw commented Oct 24, 2020

thesamprice commented Aug 25, 2021

stanislaw commented Sep 22, 2021

stanislaw commented Jan 20, 2020 •

edited

Loading

stanislaw Jan 22, 2020 •

edited

Loading

stanislaw Jan 22, 2020 •

edited

Loading

stanislaw commented Apr 24, 2020 •

edited

Loading